Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add per-operation locks to FileManager #2147

Merged

Conversation

matrss
Copy link
Collaborator

@matrss matrss commented Jan 10, 2024

Without these locks there is a race condition when writing to the operations "main.ftml" file, which happens in multiple different methods, e.g. save_file. When the mscolab application is running in a multi-threaded wsgi server it can happen that two threads try to write to the same file, corrupting it in the process.

Split off from #2100.

Without these locks there is a race condition when writing to the
operations "main.ftml" file, which happens in multiple different
methods, e.g. save_file.  When the mscolab application is running in a
multi-threaded wsgi server it can happen that two threads try to write
to the same file, corrupting it in the process.
@matrss matrss marked this pull request as ready for review January 10, 2024 17:45
@matrss matrss requested a review from ReimarBauer January 10, 2024 17:45
@@ -29,6 +29,7 @@
import difflib
import logging
import git
import threading
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please import only what we need from threading

from threading import Lock

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I prefer having it namespace'd properly where I use it and it is not like threading is excessively long. Having it just be called Lock where it is used makes it ambiguous if it is coming from threading or from multiprocessing for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, we keep it.
We have no generel style for this, usually we use import when we use lots of its methods.

in both cases the module is always fully imported (no speedup by a selection, python parses the whole module).

Copy link
Member

@ReimarBauer ReimarBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx

@ReimarBauer ReimarBauer merged commit cff4e63 into Open-MSS:develop Jan 11, 2024
4 checks passed
@matrss matrss deleted the fix-race-condition-in-file-manager branch January 11, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants